-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat: Table inline editing polish #9002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
||
### EditableCell | ||
|
||
<PropTable component={docs.exports.EditableCell} links={docs.links} showDescription /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how do i debug the PropTable.tsx component? I can't tell why placement is being added to an Overlay group in the table
Also, noticed that I cannot see the bottom of the TOC unless I scroll to the bottom of the page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
styles: style({ | ||
// TODO: really need access to display here instead, but not possible right now | ||
// will be addressable with displayOuter | ||
// Could use `hidden` attribute instead of css, but I don't have access to much of this state at the moment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still need to solve this issue
Build successful! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving for testing, but some small comments
|
||
### EditableCell | ||
|
||
<PropTable component={docs.exports.EditableCell} links={docs.links} showDescription /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
':is([role="rowheader"]:hover, [role="gridcell"]:hover)': { | ||
selectionMode: { | ||
none: colorMix('gray-25', 'gray-900', 7), | ||
single: 'gray-25', | ||
multiple: 'gray-25' | ||
} | ||
}, | ||
':is([role="row"][data-focus-visible-within] [role="rowheader"]:focus-within, [role="row"][data-focus-visible-within] [role="gridcell"]:focus-within)': 'gray-25' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar question to what @reidbarber mentioned, but more about the hover state: should there really be hover state if selection isn't supported but editing is? IMO it makes it kinda feel like there is an associated row action when there isn't. The designs aren't super clear to me if this is desired or not haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From discussions with design, yes, it should, and we'll want to open that up in RAC in the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From discussions with design, yes, it should, and we'll want to open that up in RAC in the future
I'm not sure about placement, but align is wrong, that's cell alignment, not overlay alignment |
yep, I believe so |
Co-authored-by: Daniel Lu <dl1644@gmail.com>
Build successful! 🎉 |
## API Changes
react-aria-components/react-aria-components:TooltipTrigger TooltipTrigger {
children: ReactNode
closeDelay?: number = 500
- closeOnPress?: boolean = true
defaultOpen?: boolean
delay?: number = 1500
isDisabled?: boolean
isOpen?: boolean
trigger?: 'hover' | 'focus' = 'hover'
} /react-aria-components:SearchFieldRenderProps SearchFieldRenderProps {
isDisabled: boolean
isEmpty: boolean
isInvalid: boolean
- isReadOnly: boolean
- isRequired: boolean
state: SearchFieldState
} /react-aria-components:ColumnRenderProps ColumnRenderProps {
allowsSorting: boolean
isFocusVisible: boolean
isFocused: boolean
isHovered: boolean
- isPressed: boolean
isResizing: boolean
sort: (SortDirection) => void
sortDirection: SortDirection | undefined
startResize: () => void /react-aria-components:TooltipTriggerComponentProps TooltipTriggerComponentProps {
children: ReactNode
closeDelay?: number = 500
- closeOnPress?: boolean = true
defaultOpen?: boolean
delay?: number = 1500
isDisabled?: boolean
isOpen?: boolean
trigger?: 'hover' | 'focus' = 'hover'
} /react-aria-components:RangeCalendarState-RangeCalendarState <T extends DateValue = DateValue> {
+RangeCalendarState {
anchorDate: CalendarDate | null
focusNextDay: () => void
focusNextPage: () => void
focusNextSection: (boolean) => void
focusPreviousDay: () => void
focusPreviousPage: () => void
focusPreviousRow: () => void
focusPreviousSection: (boolean) => void
focusSectionEnd: () => void
focusSectionStart: () => void
focusedDate: CalendarDate
getDatesInWeek: (number, CalendarDate) => Array<CalendarDate | null>
highlightDate: (CalendarDate) => void
highlightedRange: RangeValue<CalendarDate> | null
isCellDisabled: (CalendarDate) => boolean
isCellFocused: (CalendarDate) => boolean
isCellUnavailable: (CalendarDate) => boolean
isDisabled: boolean
isDragging: boolean
isFocused: boolean
isInvalid: (CalendarDate) => boolean
isNextVisibleRangeInvalid: () => boolean
isPreviousVisibleRangeInvalid: () => boolean
isReadOnly: boolean
isSelected: (CalendarDate) => boolean
isValueInvalid: boolean
maxValue?: DateValue | null
minValue?: DateValue | null
selectDate: (CalendarDate) => void
selectFocusedDate: () => void
setAnchorDate: (CalendarDate | null) => void
setDragging: (boolean) => void
setFocused: (boolean) => void
setFocusedDate: (CalendarDate) => void
setValue: (RangeValue<DateValue> | null) => void
timeZone: string
value: RangeValue<DateValue> | null
visibleRange: RangeValue<CalendarDate>
} @react-aria/table/@react-aria/table:TableColumnHeaderAria TableColumnHeaderAria {
columnHeaderProps: DOMAttributes
- isPressed: boolean
} @react-aria/tooltip/@react-aria/tooltip:TooltipTriggerProps TooltipTriggerProps {
closeDelay?: number = 500
- closeOnPress?: boolean = true
defaultOpen?: boolean
delay?: number = 1500
isDisabled?: boolean
isOpen?: boolean
trigger?: 'hover' | 'focus' = 'hover'
} @react-spectrum/s2/@react-spectrum/s2:EditableCell EditableCell {
align?: 'start' | 'center' | 'end' = 'start'
children: ReactNode
className?: ClassNameOrFunction<CellRenderProps> = 'react-aria-Cell'
colSpan?: number
id?: Key
isSaving?: boolean
- onCancel: () => void
onSubmit: () => void
renderEditing: () => ReactNode
showDivider?: boolean
style?: StyleOrFunction<CellRenderProps>
} /@react-spectrum/s2:TooltipTrigger TooltipTrigger {
children: ReactNode
- closeOnPress?: boolean = true
containerPadding?: number = 12
crossOffset?: number = 0
defaultOpen?: boolean
delay?: number = 1500
isOpen?: boolean
offset?: number = 0
onOpenChange?: (boolean) => void
placement?: 'start' | 'end' | 'right' | 'left' | 'top' | 'bottom' = 'top'
shouldFlip?: boolean = true
trigger?: 'hover' | 'focus' = 'hover'
} /@react-spectrum/s2:TooltipTriggerProps TooltipTriggerProps {
children: ReactNode
closeDelay?: number = 500
- closeOnPress?: boolean = true
defaultOpen?: boolean
delay?: number = 1500
isDisabled?: boolean
isOpen?: boolean
trigger?: 'hover' | 'focus' = 'hover'
} @react-spectrum/tooltip/@react-spectrum/tooltip:TooltipTrigger TooltipTrigger {
children: [ReactElement, ReactElement]
- closeOnPress?: boolean = true
containerPadding?: number = 12
crossOffset?: number = 0
defaultOpen?: boolean
delay?: number = 1500
isOpen?: boolean
offset?: number = 7
onOpenChange?: (boolean) => void
placement?: Placement = 'top'
shouldFlip?: boolean = true
trigger?: 'hover' | 'focus' = 'hover'
} /@react-spectrum/tooltip:SpectrumTooltipTriggerProps SpectrumTooltipTriggerProps {
children: [ReactElement, ReactElement]
- closeOnPress?: boolean = true
containerPadding?: number = 12
crossOffset?: number = 0
defaultOpen?: boolean
delay?: number = 1500
isOpen?: boolean
offset?: number = 7
onOpenChange?: (boolean) => void
placement?: Placement = 'top'
shouldFlip?: boolean = true
trigger?: 'hover' | 'focus' = 'hover'
} @react-stately/calendar/@react-stately/calendar:RangeCalendarState-RangeCalendarState <T extends DateValue = DateValue> {
+RangeCalendarState {
anchorDate: CalendarDate | null
focusNextDay: () => void
focusNextPage: () => void
focusNextSection: (boolean) => void
focusPreviousDay: () => void
focusPreviousPage: () => void
focusPreviousRow: () => void
focusPreviousSection: (boolean) => void
focusSectionEnd: () => void
focusSectionStart: () => void
focusedDate: CalendarDate
getDatesInWeek: (number, CalendarDate) => Array<CalendarDate | null>
highlightDate: (CalendarDate) => void
highlightedRange: RangeValue<CalendarDate> | null
isCellDisabled: (CalendarDate) => boolean
isCellFocused: (CalendarDate) => boolean
isCellUnavailable: (CalendarDate) => boolean
isDisabled: boolean
isDragging: boolean
isFocused: boolean
isInvalid: (CalendarDate) => boolean
isNextVisibleRangeInvalid: () => boolean
isPreviousVisibleRangeInvalid: () => boolean
isReadOnly: boolean
isSelected: (CalendarDate) => boolean
isValueInvalid: boolean
maxValue?: DateValue | null
minValue?: DateValue | null
selectDate: (CalendarDate) => void
selectFocusedDate: () => void
setAnchorDate: (CalendarDate | null) => void
setDragging: (boolean) => void
setFocused: (boolean) => void
setFocusedDate: (CalendarDate) => void
setValue: (RangeValue<DateValue> | null) => void
timeZone: string
value: RangeValue<DateValue> | null
visibleRange: RangeValue<CalendarDate>
} @react-stately/tooltip/@react-stately/tooltip:TooltipTriggerProps TooltipTriggerProps {
closeDelay?: number = 500
- closeOnPress?: boolean = true
defaultOpen?: boolean
delay?: number = 1500
isDisabled?: boolean
isOpen?: boolean
trigger?: 'hover' | 'focus' = 'hover'
} |
Closes
A few remaining items that I realised I'd forgotten
Adds a background colour to hovered cells.
Adds new docs example
Remove unused onCancel prop
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: